New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update: no-restricted-imports custom message for patterns (fixes #11843) #14580
Update: no-restricted-imports custom message for patterns (fixes #11843) #14580
Conversation
Hi @lexholden!, thanks for the Pull Request The first commit message isn't properly formatted. We ask that you update the message to match this format, as we use it to generate changelogs and automate releases.
Read more about contributing to ESLint here |
e80cfce
to
57af49c
Compare
Hi @lexholden!, thanks for the Pull Request The first commit message isn't properly formatted. We ask that you update the message to match this format, as we use it to generate changelogs and automate releases.
Read more about contributing to ESLint here |
57af49c
to
147ad2e
Compare
Hi @lexholden!, thanks for the Pull Request The first commit message isn't properly formatted. We ask that you update the message to match this format, as we use it to generate changelogs and automate releases.
Read more about contributing to ESLint here |
147ad2e
to
2a56777
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@lexholden thanks for the PR! This looks very good, but the schema needs some modifications. I also left some suggestions about the docs and tests.
Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>
Hi @lexholden!, thanks for the Pull Request The pull request title isn't properly formatted. We ask that you update the message to match this format, as we use it to generate changelogs and automate releases.
Read more about contributing to ESLint here |
Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>
Hi @lexholden!, thanks for the Pull Request The pull request title isn't properly formatted. We ask that you update the message to match this format, as we use it to generate changelogs and automate releases.
Read more about contributing to ESLint here |
@mdjermanovic thanks for the excellent suggestions, agree with all of them and pushed the changed. Let me know if you think of anything else. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great, I just missed in the first review that we should also add to docs some examples of incorrect/correct code with the new configuration.
It can be something like this
incorrect:
/*eslint no-restricted-imports: ["error", { patterns: [{
group: ["lodash/*"],
message: "Please use the default import from 'lodash' instead."
}]}]*/
import pick from 'lodash/pick';
correct:
/*eslint no-restricted-imports: ["error", { patterns: [{
group: ["lodash/*"],
message: "Please use the default import from 'lodash' instead."
}]}]*/
import lodash from 'lodash';
@mdjermanovic good point. Done. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks!
Thanks for contributing! |
Thank you @lexholden This is a great feature! |
Update needed to get the fix for "no-restricted-imports custom message for patterns" See PR : eslint/eslint#14580 Now we can add a custom message for */generated/graphql patterns.
…
Prerequisites checklist
What is the purpose of this pull request? (put an "X" next to an item)
[ ] Documentation update
[ ] Bug fix (template)
[ ] New rule (template)
[x] Changes an existing rule (template)
[ ] Add autofixing to a rule
[ ] Add a CLI option
[ ] Add something to the core
[ ] Other, please explain:
What changes did you make? (Give an overview)
As per the spec agreed upon in #11843 - the rule
no-restricted-imports
now allows multiple patterns to be grouped together with a message that is logged if any match failsFor example,
{ patterns: [{ group: ["foo/*", "bar/*"], message: "'foo' and 'bar' are deprecated, please use 'baz' instead." }] }
would log error'foo/some/subimport' import is restricted from being used by a pattern. 'foo' and 'bar' are deprecated, please use 'baz' instead.
Is there anything you'd like reviewers to focus on?
Wrote this for a personal project, but happy to tweak anything required to shepherd it through any part of the community process that I've missed from the contributor guidelines or add some more robust test edge cases if proposed.